Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:104">
P2: Clicking a document citation whose source was deleted silently does nothing because this handler discards `reveal`'s failure result. Handling the `false` result and showing the existing “This source is no longer available.” toast would give document citations the same feedback as chat citations.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:57">
P2: Receipt rows remain a static “Edited”/line-count display and never show the required Edited since/Undone status, so users cannot tell from the transcript that a receipt is no longer reviewable without clicking each row. Deriving and rendering the latest receipt status for each row would keep the persistent transcript accurate after undo or subsequent edits.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts:29">
P2: Edits from messages using the legacy sibling-tool representation disappear from the document-change receipt. When `action` is absent here, the compatibility path has already removed the original `workspace_edit_item` part, so preserving or deriving its document-edit action in that path would keep historical review and undo available.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread.ts:175">
P2: Documents created or edited through `orchestrate` lose workspace citation links because its nested workspace tools do not receive the new resolver. Passing the same resolver through the turn-tool configuration and Code Mode tool catalog preserves citations for both direct and orchestrated operations.</violation>

<violation number="2" location="src/features/workspaces/ai/ai-thread.ts:529">
P2: A new document can persist a stale citation from an earlier chat turn because this resolver searches all transcript records even though workspace refs are turn-scoped. Limiting lookup to `activeWorkspaceReferences` keeps old refs unresolved instead of attaching outdated source metadata.</violation>
</file>

<file name="src/features/workspaces/documents/document-html-chunk.ts">

<violation number="1" location="src/features/workspaces/documents/document-html-chunk.ts:33">
P2: A fresh start read (offset 0) of an empty document now fails instead of returning an empty/tractable result. For a document with zero blocks, `document.childCount` is 0 and the guard `offset >= document.childCount` is true for `offset: 0`, so `readDocumentHtmlChunk` returns `undefined`. In `document-session.readHtmlChunk` that becomes `{ status: "invalid_offset" }`, and the reader maps a *start* mode read (which carries no cursor at all) to `{ code: "invalid_cursor", status: "failed" }`. The AI therefore cannot read an empty document it just created — workspace_read_items reports an invalid cursor on a legitimate first read, which is misleading and blocks create-then-edit workflows on empty documents. The test case "rejects a nonzero continuation offset for an empty document" implies offset 0 should be a valid start read, but no path produces a ready result for it.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:181">
P3: Line tallies for atom blocks (rules, formulas, etc.) can be miscounted across whole-document rewrites. `countDocumentLines` keys atom lines by the full node JSON, which includes the internal `aiRef` attribute that `ensureProseMirrorDocumentAiRefs` regenerates on every top-level block during `replace_all`. As a result an atom whose visible content did not change is still reported as one line removed and one line added in the receipt tally. Consider stripping the ref (e.g. `withTiptapNodeAiRef(node, null)` before `toJSON()`, or key on the block's content) so the tally reflects visible content only, matching how text blocks are keyed by `textContent`.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:121">
P3: This schema is shared with the live client editor (`getTiptapDocumentSchemaExtensions` is spread into the editor via `tiptap-extensions.ts`), so flipping `Link` `openOnClick` from `false` to `true` changes link interaction for every document being edited, not just the server-side HTML projection used for AI editing. Clicking a link in the editor now navigates to the URL instead of keeping the cursor/selection, a user-facing behavior change in normal editing. `openOnClick` is an editor-interaction option with no effect on server parse/serialize, so unless this editor UX change is intentional for the PR it reads as an incidental side effect of the schema refactor.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:444">
P2: The undo/edit write path changed how the shared document fragment is replaced. Previously `replaceCurrentDocument` explicitly cleared the Y.XmlFragment (`fragment.delete(0, fragment.length)`) before calling `prosemirrorJSONToYXmlFragment`; the new `reconcileCurrentDocument` calls it directly on the existing fragment with no clear. If `prosemirrorJSONToYXmlFragment` merges into rather than replaces the fragment content, every applied edit, undo, and ref-reconciliation in `applyEdits`/`undoDocumentEditReceipt`/`getReferencedDocumentSnapshot` would append the full document again, compounding content. Please confirm the helper replaces its target fragment; if it does not, restore the `fragment.delete(0, fragment.length)` (and the surrounding transaction) before inserting.</violation>
</file>

<file name="src/features/workspaces/operations/document-citations.ts">

<violation number="1" location="src/features/workspaces/operations/document-citations.ts:14">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `resolveDocumentCitations` operation is introduced without unit tests, even though regression-style assertions are practical. The function has two distinct branches—no-op when refs are empty or the resolver is absent, and mapping resolved records back into HTML—that could be trivially covered with a mocked `WorkspaceAccessContext`. In a behavior-change PR, introducing untested core logic increases the risk of silent regressions in citation resolution. Adding a focused test file (e.g., `document-citations.test.ts`) would exercise both branches and document the expected behavior for future changes.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:209">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new local-persistence readiness condition that checks `session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0` before marking a cached session ready is a behavior change with no test coverage. A practical regression assertion exists (seed local persistence with the `server-synced` flag and an empty tiptap fragment, assert the session is not marked ready), yet no tests cover this path. For a behavior-change PR, new gating logic should be validated by a regression test.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentToolbar.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentToolbar.tsx:55">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior-change PR introduces new conditional render paths in DocumentToolbar (review mode, read-only mode, and a font-size→text-style rename) that are not exercised by any visible tests. Rule 1 flags behavior-change PRs where changed behavior is not tested, especially when regression-style assertions are practical. Consider adding component-level tests that verify the review controls render when activeReview.itemId matches, the minimal toolbar renders when canEdit is false, and the renamed text-style actions and icons behave correctly.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration.ts:133">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new behavior branches for model output transformation and edit-receipt metadata attachment are not covered by regression-style tests. `attachDocumentEditReceiptMetadata` has no test references at all in the test suite, and while `getAIThreadOrchestrationModelOutput` is tested as a standalone function, the `toModelOutput` property added to the tool returned by `createAIThreadOrchestrationTool` is never exercised. Since the existing worker test already mocks `createExecuteRuntime` and exercises connector tools, practical assertions could be added for both branches: one verifying that `workspace_edit_item` results carry the edit-receipt metadata, and another verifying the tool's `toModelOutput` produces the expected model-visible JSON shape.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/operations/create-items.ts">

<violation number="1" location="src/features/workspaces/operations/create-items.ts:121">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior-change PR modifies `createWorkspaceItemsOperation` to resolve citations via `resolveDocumentCitations` and parse initial content with `parseDocumentAiHtml`, yet no tests exercise the new integration or the updated failure path. A regression-style test for document creation with HTML/citations—and for the `invalid_initial_content` rejection—should be added to guard against regressions.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:50">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

The `readBudgetExhausted` flag changes batch read semantics so that once any item exceeds the byte budget, all later items are rejected without being read. Previously each item was checked independently, so a small item after a skipped large one could still be returned. This behavior change is not documented in the PR description or in code comments, and the existing test `bounds total content returned by a batch` cannot distinguish the new semantics from the old because every test item is identical. A regression-style assertion should be added (or the semantics should be reverted and documented) so that mixed-sized batches are handled correctly and items that would have returned real read errors are not masked as `read_budget_exceeded`.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:104">
P2: Clicking an inline citation whose source was deleted or is otherwise unavailable silently does nothing. Handle the `reveal` return value and surface the same unavailable-source feedback used by chat citations.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts:27">
P2: Document-edit receipts disappear for legacy assistant messages that contain a Code Mode part plus a sibling `workspace_edit_item`: legacy display grouping drops the sibling's document-edit action, and this branch skips the original part. Preserving the action when building legacy children, or retaining/processing those sibling tool parts here, keeps review and undo available for those messages.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:61">
P2: Responses with more than eight successful edits to one document cannot be reviewed or undone, even though the latest document is valid. Pass only a supported contiguous receipt suffix or change the grouping/retention contract so evicted IDs are not sent.</violation>

<violation number="2" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:70">
P2: After Undo, the transcript receipt still looks like an active edit and offers no `Undone` or `Edited since` state. Add live receipt-status state or update the row when review and undo complete.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration-contract.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration-contract.ts:147">
P2: The Code Mode model can read the document-edit receipt ID even though this projection is intended to keep UI metadata app-only. Stripping only the final result is too late; the connector should remove UI metadata before returning the nested tool result or carry the receipt through a non-model-visible side channel.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:121">
P2: This flips link behavior for the entire collaborative editor from `openOnClick: false` to `true`, so clicking any link in a document now opens it in a new tab rather than letting the user click to position the cursor and edit it. That is a cross-cutting change to every document edit surface, while the PR's feature is about the read-only review/undo workflow where clickable links make sense. If the intent was only to make links open in the review window, scope the behavior there; otherwise confirm this global editor change is intended.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-review-overlay.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:45">
P1: If loading the review fails or the server reports the changes are no longer reviewable, the document editor is left permanently read-only. The effect freezes the editor with `setEditable(false)` up front, but the `isError` and `status !== "ready"` branches return early without scheduling any cleanup, and the only `setEditable(canEdit)` restore lives in the success-path cleanup. When `hideReview()` clears the active review, React re-runs the effect with `target === null`, and since no cleanup was registered the editor stays frozen. Recommend restoring editability on the failure paths (for example, call `editor.setEditable(canEdit)` before `hideReview()` in both early-return branches, or always return a cleanup that resets editability).</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/document-editor/document-editor-state.ts">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-state.ts:169">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior change adds heading level 4 detection and renames the state union from `DocumentFontSize` to `DocumentTextStyle`, but no tests in the suite exercise `getActiveBlock` or the new `textStyle` values. A practical regression test (for example, mocking a TipTap editor and asserting the returned block for each heading level and the paragraph default) would prevent silent breakage of the toolbar state. Please add unit tests that cover the new `heading4` branch and the semantic rename.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The toolbar semantics changed from font-size-based actions to text-style-based actions (paragraph and heading levels 1–4), including a new Heading 4 action with its own `active` state, `run` command, and icon mapping. However, there is no visible test coverage exercising this new behavior. Consider adding unit tests that assert the `isTextStyle` active-state logic, the `getTextStyleIcon` mappings, and the TipTap commands invoked by each `documentTextStyleActions` entry—including the new heading level 4—to prevent regressions in the document editor toolbar.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:207">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The modified persistence-based readiness condition introduces a real behavior change, but there's no visible test coverage for it. A session that has the cached 'server-synced' marker in IndexedDB but an empty tiptap XML fragment will no longer be marked ready early and will instead wait for the websocket sync event. This edge case is now silently altered without regression protection. Consider adding a test that mocks the IndexedDB marker and toggles an empty vs. non-empty `Y.XmlFragment` to assert whether `markReady()` fires without a websocket sync — this directly exercises the change and guards against regressions.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:92">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR adds new user-visible citation-click navigation behavior, but no corresponding tests were found that exercise this interaction. Rule 1 (Flag AI Slop and Fabricated Changes) specifically calls out behavior-change PRs where changed behavior is not exercised by tests, especially when a regression-style assertion is practical. The click handler extracts `data-item-id` and `data-page`, parses the page conditionally, and calls `reveal()` — all of which is testable. Similarly, the newly wired `useDocumentEditReviewOverlay` hook lacks test coverage. Consider adding tests that verify the navigation logic and overlay integration to prevent regressions.</violation>

<violation number="2" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:103">
P3: Clicking a citation that points at an item that has since been deleted or renamed silently does nothing. The new click handler always calls event.preventDefault() and returns true as if the citation was actioned, but it discards the boolean reveal() returns (workspace-location-context.tsx returns Boolean(viewInstanceId), which is false when the target item no longer exists). Because the new styles.css gives citations cursor:pointer, a stale citation still looks clickable but produces no navigation and no feedback. Consider checking reveal()'s result (or hasItem) and, when it returns false, falling back to opening the plain document item or otherwise signalling that the source is unavailable, rather than swallowing the click.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration.ts:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior-change PR adds new orchestration wiring (`toModelOutput` on the orchestration tool and receipt-metadata attachment for `workspace_edit_item`) without tests that exercise these exact code paths. The related output-normalization helpers are tested, but the wiring that connects them into the tool lifecycle is not. A regression-style test should assert that `createAIThreadOrchestrationTool` returns a tool whose `toModelOutput` produces the expected compact projection, and that a `workspace_edit_item` invocation through the connector carries receipt metadata keyed by `invocationId`.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread.ts:175">
P1: Document citations are lost when the assistant edits through `orchestrate`: the resolver is wired into direct workspace tools here, but not into the separately constructed Code Mode workspace tools. Thread the resolver through `createAIThreadTurnToolConfig` and its tool catalog as well so both execution paths resolve `<citation ref>` locations.</violation>
</file>

<file name="src/features/workspaces/operations/edit-item.ts">

<violation number="1" location="src/features/workspaces/operations/edit-item.ts:84">
P2: A batch with multiple citation-bearing HTML edits now rereads the chat transcript once per edit, creating up to 40 concurrent Durable Object reads for one tool call and avoidable latency/load. Resolving the union of citation refs once and reusing the records for each HTML fragment would preserve behavior without the per-edit read amplification.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration-contract.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration-contract.ts:208">
P3: The model-facing boundary strips app-only `action` from calls but does not strip the `__thinkexUi`/receipt metadata from `result` — it only stays clean because `execute` happens to feed it already-normalized (pre-stripped) output. If this exported helper is ever called with the raw runtime output (its signature accepts any `output`), the internal receipt ID would surface to the model. Applying `stripAIThreadToolUiMetadata` to `result` here (as `normalizeAIThreadOrchestrationOutput` already does) would make the boundary self-contained and consistent.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:443">
P2: `reconcileCurrentDocument` now repopulates an existing Yjs `XmlFragment` without clearing it first, whereas the previous `replaceCurrentDocument` explicitly did `fragment.delete(0, fragment.length)` and wrapped the write in `this.document.transact(...)`. If `prosemirrorJSONToYXmlFragment` appends (as the old code's explicit delete strongly suggests), every AI edit and every ref reconciliation would duplicate the document's top-level blocks, and dropping the `transact` wrapper also removes the atomicity the old code guaranteed when streaming updates to connected clients. Please verify the library clears the fragment and restore the explicit clear/transaction (as in the removed code) if it does not.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-html.test.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-html.test.ts:23">
P3: The round-trip test never verifies the round-trip: it only checks substrings and that re-parsing yields `type: "doc"`, so silently dropped or reordered blocks still pass despite the test name and the PR's "remains lossless" claim. Consider asserting the re-parsed document deep-equals the original (or a normalized snapshot) to actually guard the round-trip.</violation>
</file>

<file name="src/features/workspaces/operations/document-citations.ts">

<violation number="1" location="src/features/workspaces/operations/document-citations.ts:26">
P2: Model-supplied citation metadata can survive resolution: an unresolved ref can retain a fabricated `data-item-id`, and an item citation can retain a fabricated `data-page`. Strip server-owned citation attributes from ref-bearing tags before applying locations so only a resolved workspace record supplies them.</violation>

<violation number="2" location="src/features/workspaces/operations/document-citations.ts:28">
P2: A turn-local ref can resolve to the wrong workspace item when the same short ref appears in more than one transcript turn, because the map overwrites earlier records without checking their locations. Preserve only unambiguous refs (matching the collision handling used by `workspace-citations`) before writing the citation.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-review-overlay.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:46">
P1: When a review fails to load or the server reports the receipt as unavailable (not_found, content_changed, not_latest, reverted, review_unavailable), the document editor is left permanently frozen in read-only mode. The effect calls `editor.setEditable(false)` up front, but the error and 'not ready' branches call `hideReview()` and `return` before any cleanup is registered; since `hideReview()` nulls the target, the effect re-runs and exits through the `if (!editor || !target) return;` guard without ever restoring `editor.setEditable(canEdit)`. The user is then stuck unable to edit that document until it is remounted. Consider restoring editability in those early-return branches (or always registering a cleanup that restores it) before calling hideReview().</violation>
</file>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/workspaces/ai/ai-thread-runtime.ts`:
- Line 164: Update src/features/workspaces/ai/ai-thread-runtime.ts at line 164
so createAIThreadTurnToolConfig accepts and forwards resolveWorkspaceReferences
to createAIThreadToolCatalog; update src/features/workspaces/ai/ai-thread.ts at
line 175 so _createTurnToolConfig passes this._resolveWorkspaceReferences into
createAIThreadTurnToolConfig.

In `@src/features/workspaces/documents/document-ai-html.ts`:
- Around line 267-270: Update the element metadata handling in
parseDocumentAiHtml so non-"pdf-page" locations remove any existing data-page
attribute after setting data-item-id. Preserve setting data-page from
location.pageNumber for PDF-page citations, ensuring item-level citations do not
retain stale page values.

In `@src/features/workspaces/documents/document-edit-review-context.tsx`:
- Around line 44-69: Update the showReview callback to track a monotonically
increasing request identifier for each invocation, and capture the current
identifier before awaiting getDocumentEditReceiptReviewFn. After the request
resolves, ignore the result—including errors and status handling—unless its
identifier is still the latest, so an older request cannot call setActiveReview
or display stale feedback over a newer selection.

In `@src/styles.css`:
- Around line 668-684: Update the `.workspace-document-prose citation` and
corresponding `citation:hover` selectors to target the class or data attribute
emitted by the citation renderer instead of the unknown `citation` type
selector. Ensure both rules preserve their existing styles and satisfy Stylelint
without broad exceptions.

---

Nitpick comments:
In `@src/styles.css`:
- Around line 668-684: Update the citation interaction represented by the
.workspace-document-prose citation styles to use a keyboard-focusable link or
button with keyboard activation, and add a clear :focus-visible state matching
the hover treatment. If citations are not actually interactive, remove cursor:
pointer and the citation:hover rule instead.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:181">
P2: The receipt's line tally counts atom blocks (horizontal rule, math) by stringifying `node.toJSON()`, which includes the top-level `aiRef` attribute on every block. Because the `aiRef` fingerprint is part of the serialized line, an unchanged formula/hr block whose ref was regenerated by a `replace_all` edit will be counted as both a line removed and a line added, inflating the receipt's "Lines: +N −M" totals even though the block's content never changed. This is inconsistent with the text-block branch, which strips refs by using only `node.textContent`. Consider normalizing refs away before stringifying the atom, matching the `withoutTopLevelAiRefs`/`withTiptapNodeAiRef(node, null)` approach used elsewhere.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-html.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-html.ts:67">
P3: An unresolved citation (no data-item-id) that sits at the document root gets replaced by a plain-text node before validation, and then validateDocumentAiHtml rejects any non-whitespace top-level text node as "Plain text and Markdown are not accepted." So at the root level the documented design ("becomes its own label rather than failing the write") doesn't hold — the edit still fails, and with a misleading non-HTML message. If a resolver-less citation should degrade to its label, consider wrapping it in a paragraph (or skipping the top-level text check for known-good replacements) so the write doesn't fail here.</violation>

<violation number="2" location="src/features/workspaces/documents/document-ai-html.ts:268">
P2: When a citation already has `data-page` and the resolved location is not `pdf-page`, this branch leaves the old page attribute in place. That stale metadata can make an item-level citation look page-specific on later parse/read cycles. Clearing `data-page` in the non-PDF case would prevent that carry-over.</violation>
</file>

<file name="src/features/workspaces/operations/create-items.ts">

<violation number="1" location="src/features/workspaces/operations/create-items.ts:121">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The `createWorkspaceItemsOperation` function now resolves citations via `resolveDocumentCitations` and parses `initialContent` as HTML through `parseDocumentAiHtml` (replacing the previous Markdown path). This is a meaningful behavior change: non-HTML content that was previously tolerated is now rejected as `invalid_initial_content`, and the `warnings` field has been removed entirely. However, there are no tests exercising this changed behavior in `create-items.ts`. Practical regression assertions — e.g., validating that HTML content succeeds, non-HTML returns `invalid_initial_content`, and citations are resolved during creation — are missing. Adding tests for the create-items path would ensure the new parsing and citation behavior is properly validated.</violation>

<violation number="2" location="src/features/workspaces/operations/create-items.ts:303">
P2: Created documents can contain fabricated citation targets: an AI-supplied `<citation data-item-id="...">` bypasses `resolveDocumentCitations` and is persisted as a clickable citation without a verified workspace reference. Normalize or reject `data-item-id` in model HTML and only retain IDs produced by resolving a valid `ref`.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:209">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR changes the document-session readiness logic by adding an extra `session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0` check to the IndexedDB cache path. Previously, a cached `server-synced` flag alone would mark the session ready; now an empty Yjs fragment prevents that. If a user is offline with an empty or newly-created previously-synced document, the session will never become ready because the provider `sync` handler won't fire and the IndexedDB cache path is now gated. This is a real behavior change with a clear offline regression, yet no tests cover this file or the new condition. Please add a regression test asserting that a non-empty fragment with a cached sync flag marks ready, while an empty fragment with the same flag defers readiness.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx:79">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `heading4` toolbar action and the broader refactor from `fontSize` to `textStyle` block kinds are user-visible behavior changes, yet no tests cover these additions. The `document-editor` component directory has no test files at all, and no regression-style assertions verify that `documentTextStyleActions` includes `heading4`, that `getTextStyleIcon` returns the correct icon, or that `isTextStyle` matches the new `textStyle` kind. Practical unit tests for these exports would be straightforward and should be added to guard against future regressions.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts:7">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `ai-thread-tool-ui-metadata.ts` module introduces runtime behavior for attaching, reading, and stripping AI tool output metadata, but has no test coverage. Regression-style assertions are very practical for these pure utilities (attach→get round-trip, recursive strip at various nesting depths, pass-through for non-objects). Please add unit tests for `attachDocumentEditReceiptMetadata`, `getDocumentEditReceiptMetadata`, and `stripAIThreadToolUiMetadata`.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration.ts:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR adds a new `toModelOutput` property to the orchestration tool that wraps `getAIThreadOrchestrationModelOutput`, but no test verifies that the returned tool actually includes this property or that it returns the expected `{ type: "json", value: ... }` shape. While the underlying helper is tested separately, the integration point is not. A regression assertion here is practical and would prevent silent breakage of the model-output projection.</violation>

<violation number="2" location="src/features/workspaces/ai/ai-thread-orchestration.ts:133">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The connector now conditionally attaches document-edit receipt metadata for `workspace_edit_item`, but this execution path is not exercised by any visible test. The `ai-thread-orchestration.worker.test.ts` mocks `createExecuteRuntime` entirely and only tests a generic "nested" tool through `connector.tools()`, not `workspace_edit_item`. `attachDocumentEditReceiptMetadata` has zero references in test files. A practical regression test would exercise `workspace_edit_item` through the connector and assert receipt metadata is present, while verifying other tools do not receive it.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-extension.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-extension.ts:74">
P3: While a review is open, every document transaction (including each remote collaborative edit) replays a full-document diff: createDocumentEditReviewDecorations re-parses the entire before-document JSON into a node and runs a whole-document ChangeSet comparison against the current content. For long documents with frequent collaborators this is repeated O(doc-size) work that could be done incrementally or memoized (cache the parsed before-node/schema outside the per-transaction path). Low risk for small docs, but worth guarding given the feature is explicitly scoped around larger documents elsewhere (review_unavailable).</violation>

<violation number="2" location="src/features/workspaces/documents/document-edit-review-extension.ts:167">
P2: Changes to atom blocks nested inside lists or blockquotes can appear with no review decoration, leaving users unable to see those AI edits. Traversing descendant nodes when finding changed atoms would preserve the intended block marker for nested content.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:98">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The `readBudgetExhausted` flag changes the semantics of batch reading: after the first item exceeds the remaining byte budget, every later item is rejected without being read, even if it would fit. The PR does not document this behavior change, and the existing budget test (`bounds total content returned by a batch`) creates identical oversized items so it passes under both the old and new behavior. There is no test asserting the new hard-stop semantics, which means a small item after an oversized one is silently dropped. Consider either restoring the per-item budget check so smaller trailing items are still evaluated, or adding an explicit test that documents and asserts the new stop-after-first-rejection behavior.</violation>

<violation number="2" location="src/features/workspaces/content/workspace-content-reader.ts:107">
P3: The relations RPCs are now started without being awaited and only consumed in attachRelationPaths. In the normal path this is a fine parallelization. However, if a later read in the loop throws a non-WorkspacePageSelectionError (which is rethrown and aborts before attachRelationPaths runs), the in-flight listItemRelations promises for earlier ready items are left un-awaited and could surface as unhandled promise rejections if they reject. Consider collecting the promises and ensuring they are always awaited or attached a rejection handler so no kernel rejection is left dangling.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-context.tsx">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-context.tsx:52">
P3: showReview races when the user opens reviews for two documents in quick succession. The RPC for the first document can resolve after the second one's, and setActiveReview simply overwrites, so the visible review and the on-screen document can end up out of sync (the overlay only applies when activeReview.itemId matches the mounted document). Consider guarding the async application—e.g., keep a ref of the most recently requested itemId and ignore any response that isn't for it, or abort/cancel the prior request before issuing a new one.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:442">
P1: The new `reconcileCurrentDocument` replaces the old `replaceCurrentDocument`, but it drops the explicit `fragment.delete(0, fragment.length)` (and the `transact` wrapper) that the previous implementation performed before calling `prosemirrorJSONToYXmlFragment`. Since every caller of this helper (applyEdits, undoDocumentEditReceipt, getReferencedDocumentSnapshot) writes into a fragment that already holds content, correct replacement depends on `prosemirrorJSONToYXmlFragment` clearing the target fragment itself — which is not guaranteed by the library contract. If it merely appends/inserts, each AI edit or undo would silently duplicate the document content. Please confirm the library behavior and, if it does not clear, restore an explicit piecewise clear (ideally inside a transaction, as before) before merging.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-functions.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-functions.ts:23">
P2: This review RPC is a GET but can carry a large payload. The schema allows up to 40 receipt ids, each up to 512 characters, and TanStack Start encodes the validator data into the URL for GET requests, so a receipt-heavy turn can push the query string toward a size that trips intermediary URL-length limits and fail to load the review silently. Using method: "POST" (undo already is) avoids the URL-size constraint, or the caps can be tightened.</violation>
</file>